feat(ui): add scroll-to-bottom button and improve scroll navigation#1491
feat(ui): add scroll-to-bottom button and improve scroll navigation#1491mergify[bot] merged 1 commit intomainfrom
Conversation
…n session view Add a scroll-to-bottom button alongside the existing scroll-to-top button in the session message view. Both buttons now animate smoothly, have tooltips, and correctly hide when content doesn't overflow. Fixes ghost tooltip appearing when buttons are hidden. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
📝 WalkthroughWalkthroughThis PR adds bidirectional scroll navigation to the messages tab. The scroll detection logic is fixed to handle non-overflowing content, and the UI is updated to display both scroll-to-top and scroll-to-bottom buttons with tooltips and smooth scrolling behavior. Accompanying specification documents the requirements. ChangesBidirectional Scroll Navigation
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/frontend/src/components/session/MessagesTab.tsx`:
- Around line 294-324: The two floating buttons (the one using
showScrollToTop/scrollToTop and the one using isAtBottom/messagesContainerRef)
remain in the tab order when visually hidden; update each Button to be
unfocusable and hidden from assistive tech when its container is hidden by
adding conditional attributes like tabIndex={showScrollToTop ? 0 : -1} (and for
the other button tabIndex={isAtBottom ? -1 : 0}) and
aria-hidden={showScrollToTop ? "false" : "true"} (and aria-hidden for the other
using isAtBottom), or alternatively set disabled when hidden; apply these
changes on the Button elements referenced (the Button inside the TooltipTrigger
for scrollToTop and the Button that scrolls to bottom) so keyboard users cannot
focus hidden controls and tooltips won’t be exposed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1a58254d-0078-4667-9a59-e5304b6adbbb
📒 Files selected for processing (2)
components/frontend/src/components/session/MessagesTab.tsxspecs/frontend/sessions/messages/scroll-navigation.spec.md
| <div className={`transition-all duration-200 ${showScrollToTop ? "opacity-100 scale-100" : "opacity-0 scale-75 pointer-events-none"}`}> | ||
| <Tooltip open={showScrollToTop ? undefined : false}> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| variant="outline" | ||
| size="icon-sm" | ||
| onClick={scrollToTop} | ||
| aria-label="Scroll to top" | ||
| className="rounded-full shadow-md cursor-pointer" | ||
| > | ||
| <ChevronUp className="h-4 w-4" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="left">Scroll to top</TooltipContent> | ||
| </Tooltip> | ||
| </div> | ||
| <div className={`transition-all duration-200 ${!isAtBottom ? "opacity-100 scale-100" : "opacity-0 scale-75 pointer-events-none"}`}> | ||
| <Tooltip open={isAtBottom ? false : undefined}> | ||
| <TooltipTrigger asChild> | ||
| <Button | ||
| variant="outline" | ||
| size="icon-sm" | ||
| onClick={() => messagesContainerRef.current?.scrollTo({ top: messagesContainerRef.current.scrollHeight, behavior: "smooth" })} | ||
| aria-label="Scroll to bottom" | ||
| className="rounded-full shadow-md cursor-pointer" | ||
| > | ||
| <ChevronDown className="h-4 w-4" /> | ||
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side="left">Scroll to bottom</TooltipContent> | ||
| </Tooltip> |
There was a problem hiding this comment.
Make the hidden buttons unfocusable.
pointer-events-none only blocks mouse input. Both buttons still stay in the tab order while invisible, so keyboard users can land on hidden controls and potentially surface the tooltip.
Fix
- <Button
+ <Button
variant="outline"
size="icon-sm"
onClick={scrollToTop}
aria-label="Scroll to top"
+ tabIndex={showScrollToTop ? 0 : -1}
+ aria-hidden={!showScrollToTop}
className="rounded-full shadow-md cursor-pointer"
>
...
- <Button
+ <Button
variant="outline"
size="icon-sm"
onClick={() => messagesContainerRef.current?.scrollTo({ top: messagesContainerRef.current.scrollHeight, behavior: "smooth" })}
aria-label="Scroll to bottom"
+ tabIndex={!isAtBottom ? 0 : -1}
+ aria-hidden={isAtBottom}
className="rounded-full shadow-md cursor-pointer"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className={`transition-all duration-200 ${showScrollToTop ? "opacity-100 scale-100" : "opacity-0 scale-75 pointer-events-none"}`}> | |
| <Tooltip open={showScrollToTop ? undefined : false}> | |
| <TooltipTrigger asChild> | |
| <Button | |
| variant="outline" | |
| size="icon-sm" | |
| onClick={scrollToTop} | |
| aria-label="Scroll to top" | |
| className="rounded-full shadow-md cursor-pointer" | |
| > | |
| <ChevronUp className="h-4 w-4" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="left">Scroll to top</TooltipContent> | |
| </Tooltip> | |
| </div> | |
| <div className={`transition-all duration-200 ${!isAtBottom ? "opacity-100 scale-100" : "opacity-0 scale-75 pointer-events-none"}`}> | |
| <Tooltip open={isAtBottom ? false : undefined}> | |
| <TooltipTrigger asChild> | |
| <Button | |
| variant="outline" | |
| size="icon-sm" | |
| onClick={() => messagesContainerRef.current?.scrollTo({ top: messagesContainerRef.current.scrollHeight, behavior: "smooth" })} | |
| aria-label="Scroll to bottom" | |
| className="rounded-full shadow-md cursor-pointer" | |
| > | |
| <ChevronDown className="h-4 w-4" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="left">Scroll to bottom</TooltipContent> | |
| </Tooltip> | |
| <div className={`transition-all duration-200 ${showScrollToTop ? "opacity-100 scale-100" : "opacity-0 scale-75 pointer-events-none"}`}> | |
| <Tooltip open={showScrollToTop ? undefined : false}> | |
| <TooltipTrigger asChild> | |
| <Button | |
| variant="outline" | |
| size="icon-sm" | |
| onClick={scrollToTop} | |
| aria-label="Scroll to top" | |
| tabIndex={showScrollToTop ? 0 : -1} | |
| aria-hidden={!showScrollToTop} | |
| className="rounded-full shadow-md cursor-pointer" | |
| > | |
| <ChevronUp className="h-4 w-4" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="left">Scroll to top</TooltipContent> | |
| </Tooltip> | |
| </div> | |
| <div className={`transition-all duration-200 ${!isAtBottom ? "opacity-100 scale-100" : "opacity-0 scale-75 pointer-events-none"}`}> | |
| <Tooltip open={isAtBottom ? false : undefined}> | |
| <TooltipTrigger asChild> | |
| <Button | |
| variant="outline" | |
| size="icon-sm" | |
| onClick={() => messagesContainerRef.current?.scrollTo({ top: messagesContainerRef.current.scrollHeight, behavior: "smooth" })} | |
| aria-label="Scroll to bottom" | |
| tabIndex={!isAtBottom ? 0 : -1} | |
| aria-hidden={isAtBottom} | |
| className="rounded-full shadow-md cursor-pointer" | |
| > | |
| <ChevronDown className="h-4 w-4" /> | |
| </Button> | |
| </TooltipTrigger> | |
| <TooltipContent side="left">Scroll to bottom</TooltipContent> | |
| </Tooltip> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/frontend/src/components/session/MessagesTab.tsx` around lines 294
- 324, The two floating buttons (the one using showScrollToTop/scrollToTop and
the one using isAtBottom/messagesContainerRef) remain in the tab order when
visually hidden; update each Button to be unfocusable and hidden from assistive
tech when its container is hidden by adding conditional attributes like
tabIndex={showScrollToTop ? 0 : -1} (and for the other button
tabIndex={isAtBottom ? -1 : 0}) and aria-hidden={showScrollToTop ? "false" :
"true"} (and aria-hidden for the other using isAtBottom), or alternatively set
disabled when hidden; apply these changes on the Button elements referenced (the
Button inside the TooltipTrigger for scrollToTop and the Button that scrolls to
bottom) so keyboard users cannot focus hidden controls and tooltips won’t be
exposed.
Merge Queue Status
This pull request spent 11 seconds in the queue, including 1 second running CI. Required conditions to merge |
Summary
Extends #1483. Related to #1482.
openprop)specs/frontend/sessions/messages/scroll-navigation.spec.mdTest plan
🤖 Generated with Claude Code